Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix GCB verification with git material source prefix #519

Merged
merged 2 commits into from
Mar 9, 2023

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Mar 8, 2023

The most recent GCB build I've pushed uses a git+ prefix on the source. Updates slsa-verifier to allow these for matching against expected provenance.

See this failing run: https://github.com/slsa-framework/example-package/actions/runs/4368807186/jobs/7641862328#step:5:205

cc @laurentsimon @ianlewis

@laurentsimon
Copy link
Contributor

laurentsimon commented Mar 8, 2023

@khalkie not sure if this was flagged as a potential breaking change. Shall we work on better integration between GCB and this repo? Maybe running the slsa-verifier on as pre-submit would catch these in the future?

@khalkie
Copy link
Contributor

khalkie commented Mar 10, 2023

Sorry, my fault - the materials without "git+" were failing the BCID verifier, so I assumed they were required for SLSA as well. I did have it in my test provenance files, and added it to several unit tests - I thought I was testing the end-to-end validation in the VerifyTestProvenance test but I missed this case. It's a good idea to incorporate the SLSA verifier into our release tests

@laurentsimon
Copy link
Contributor

np, it happens. During the review, we missed that the tests were missing too!
Let's improve our collaboration to have slsa-verifier run on GCB code changes, and it should catch regression problems in the future.

Thanks @khalkie again for the contribution, much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants